Conversation
1d70297 to
06fb5bf
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR refactors configuration management by introducing a type-safe GooseMode enum and streamlining the config setter API. It replaces string-based mode comparisons with strongly-typed enum variants and updates all setters to accept generic serializable values instead of requiring pre-serialized serde_json::Value instances.
- Introduced
GooseModeenum to replace string-based mode handling - Updated
set_paramandset_secretmethods to accept generic serializable types - Added
declare_param!macro to generate type-safe getter/setter methods for common config parameters
Reviewed Changes
Copilot reviewed 30 out of 30 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/goose/src/config/goose_mode.rs | New file defining the GooseMode enum with Auto, Approve, SmartApprove, and Chat variants |
| crates/goose/src/config/base.rs | Updated config methods to use generics and added macro-generated getters/setters for GOOSE_MODE, GOOSE_PROVIDER, and GOOSE_MODEL |
| crates/goose/src/config/mod.rs | Exported the new GooseMode enum |
| crates/goose/src/agents/agent.rs | Replaced string-based mode handling with GooseMode enum |
| crates/goose/src/permission/permission_inspector.rs | Updated to use GooseMode instead of strings |
| crates/goose/src/providers/ollama.rs | Updated mode checking to use GooseMode enum |
| crates/goose/src/providers/claude_code.rs | Updated mode checking and removed obsolete test |
| crates/goose/src/providers/githubcopilot.rs | Simplified secret setting calls |
| crates/goose/src/config/signup_tetrate/mod.rs | Replaced serde_json::Value calls with direct value passing |
| crates/goose/src/config/signup_openrouter/mod.rs | Replaced serde_json::Value calls with direct value passing |
| crates/goose-cli/src/commands/configure.rs | Updated to use GooseMode enum and simplified config setter calls |
| crates/goose-cli/src/session/mod.rs | Updated mode validation to use GooseMode::try_from |
| Multiple other files | Updated config setter calls to pass values directly instead of wrapping in Value |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| pub fn should_enabled_subagents(model_name: &str) -> bool { | ||
| let config = crate::config::Config::global(); | ||
| let is_autonomous = config.get_param("GOOSE_MODE").unwrap_or("auto".to_string()) == "auto"; | ||
| let is_autonomous = config.get_goose_mode().unwrap_or(GooseMode::Auto) == GooseMode::Auto; |
There was a problem hiding this comment.
here and elsewhere; having a typed get_goose_mode is of course so much better, but should we add a default value to the pattern? we never read goose mode without defaulting to auto if it is not set
There was a problem hiding this comment.
yeah, we should do that. every config needs some kind of default value (it might be None of course)
* 'main' of github.com:block/goose: (81 commits) nextcamp - fix session resume when navigating back to chat in sidebar (#5370) feat/fix: set optional config params, and don't overwrite unset secrets (#5325) Stringly typed config (#5463) Fix: Compaction client <-> server sync (#5481) docs: recipe activity parameter substitution (#5462) only run fork on branch PRs (#5461) docs: video on goose with apify mcp (#5472) Clear windows and fix build failure (#5452) Add menu option for setting window always on top (#5429) Delete environment variable (#5479) chore: upgrade rmcp to 0.8.3 (#5458) docs: add "Building Custom Tools and Extensions for Goose" (#5469) Doc (Blog): Managing goose Configurations Across Multiple Projects (#5467) apify doc fix (#5460) Stream token usage on every agent message (#5342) rpm install in /opt/Goose to avoid conflicts with chrome-sandbox (#5421) Don't disable extensions after they fail to activate in new chat session (#5464) Add OTLP logs layer (#5386) openapi to locust load test generator recipe (#5447) technical debt tracker recipe (#5451) ... # Conflicts: # ui/desktop/src/components/ChatInput.tsx
* main: (45 commits) Change Recipes Test Script (#5457) Goose recover (#5450) don't start the default provider (#5351) keep the order of keys in config.yaml (#5468) Removed drafts and agentIsReady in ChatInput (#5366) nextcamp - fix session resume when navigating back to chat in sidebar (#5370) feat/fix: set optional config params, and don't overwrite unset secrets (#5325) Stringly typed config (#5463) Fix: Compaction client <-> server sync (#5481) docs: recipe activity parameter substitution (#5462) only run fork on branch PRs (#5461) docs: video on goose with apify mcp (#5472) Clear windows and fix build failure (#5452) Add menu option for setting window always on top (#5429) Delete environment variable (#5479) chore: upgrade rmcp to 0.8.3 (#5458) docs: add "Building Custom Tools and Extensions for Goose" (#5469) Doc (Blog): Managing goose Configurations Across Multiple Projects (#5467) apify doc fix (#5460) Stream token usage on every agent message (#5342) ...
* 'main' of github.com:block/goose: (61 commits) [Autovisualiser] remove unnecessary content from mermaid HTML template (#5505) Improve subagents docs (#5484) FIX: prefer linux in WSL and add INSTALL_OS override for CLI (#5215) Propagate session ID in LLM and MCP requests (#5165) feat: YT Short for Canva MCP + goose (#5495) Change Recipes Test Script (#5457) Goose recover (#5450) don't start the default provider (#5351) keep the order of keys in config.yaml (#5468) Removed drafts and agentIsReady in ChatInput (#5366) nextcamp - fix session resume when navigating back to chat in sidebar (#5370) feat/fix: set optional config params, and don't overwrite unset secrets (#5325) Stringly typed config (#5463) Fix: Compaction client <-> server sync (#5481) docs: recipe activity parameter substitution (#5462) only run fork on branch PRs (#5461) docs: video on goose with apify mcp (#5472) Clear windows and fix build failure (#5452) Add menu option for setting window always on top (#5429) Delete environment variable (#5479) ...
Signed-off-by: fbalicchia <fbalicchia@gmail.com>
Signed-off-by: Blair Allan <Blairallan@icloud.com>
Uh oh!
There was an error while loading. Please reload this page.